Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Fix arrow functions inside ternary expressions #408

Merged
merged 6 commits into from
Aug 23, 2016
Merged

Fix arrow functions inside ternary expressions #408

merged 6 commits into from
Aug 23, 2016

Conversation

MaximSokolov
Copy link
Contributor

Fixes #406 by using begin/end matching for ternary expressions

result = condition ? something : (a, b) => a + b

Fixes #279 by matching case statements only in switch block (replaces #405 fix)

switch (foo())
{
    case 'string':
    case a:
    case a + b:
    case foo(bar):
    case ABC:
    case 123:
      foo(bar)
      break;
    case null:
    case true:
    default:
}
case abc: // won't be highlighted

@Alhadis
Copy link
Contributor

Alhadis commented Aug 20, 2016

Uhm, what was wrong with my fix, exactly?

@Alhadis
Copy link
Contributor

Alhadis commented Aug 20, 2016

There's no need to restrict #279's fix to switch blocks, because case statements won't appear anywhere else. I feel your approach is over-complicating things.

@MaximSokolov
Copy link
Contributor Author

This is a necessary step to recognize object keys. See #307

// for e.g.
switch() {
  default:
}
a = {
  default: 123
}

@Alhadis
Copy link
Contributor

Alhadis commented Aug 20, 2016

Sigh... I see this was already discussed.

In which case, I'll probably have to rebase against your branch, since I'm in the middle of fixing numerous bugs for another PR...

@Alhadis
Copy link
Contributor

Alhadis commented Aug 22, 2016

@50Wliu Any chance of getting this merged soon? There'll be a new version of Linguist this week, and it'd be nice to have these highlighting bugs fixed on the site. Otherwise, they'll linger on GitHub for a few weeks longer. :p

Just to be clear: once this and the other PR get merged, I'll go balls out on remaining bug fixes in the hopes of getting everything under the one release. =)

/cc @pchaigno Paging you because @arfon won't hear me from where I'm standing. Literally (his name's not showing up in my mentions menu...).

@winstliu
Copy link
Contributor

Any chance of getting this merged soon?

I am currently much busier than I was just a week ago, so expect my activity on GitHub to tank significantly for a while. I'll try to merge this to unblock your other PRs but those might sit for some time before proper review.

'name': 'keyword.operator.ternary.js'
'patterns': [
{
'match': '(\\w+)(?=\\s*:)'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain what this match does in more detail?

In addition, why is the second $self needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 157241d. There is no need to include $self here. Though most of the rules must be moved to the repo and it also requires addition testing, so I kept it as is for now. The second $self includes everything else between ? and :

@winstliu
Copy link
Contributor

7296d5d yesssss

Glad to see you're working on this again @MaximSokolov!

@winstliu winstliu merged commit 0b4b99c into atom:master Aug 23, 2016
@MaximSokolov MaximSokolov deleted the fix-arrow-functions-inside-teranry-exp branch August 23, 2016 16:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants